front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx (2)

554-641: ⚠️ Potential issue | 🟠 Major

Preserve server-returned params after local poll.

When response.device is returned with updated params, the code still rebuilds newParams from the pre-poll params array (Line 607), which can drop fresh server-side params that may have been updated during the poll operation.

,

💡 Suggested fix

-      const newParams = [...params];
+      const baseDevice = latestDevice || currentDevice;
+      const newParams = Array.isArray(baseDevice.params) ? [...baseDevice.params] : [...params];
       if (usedProtocol) {
         const protocolIndex = newParams.findIndex(param => param.name === 'PROTOCOL_VERSION');
         if (protocolIndex >= 0) {
           newParams[protocolIndex] = { ...newParams[protocolIndex], value: usedProtocol };
         } else {
           newParams.push({ name: 'PROTOCOL_VERSION', value: usedProtocol });
         }
       }
-      const baseDevice = latestDevice || currentDevice;
       this.setState({
         device: {
           ...baseDevice,
           params: newParams
         },

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx` around lines 554 -
641, The pollLocal handler rebuilds newParams from the pre-poll params array,
which discards any updated params returned in response.device; change the logic
to start newParams from latestDevice.params if available (fallback to current
params) and then apply the PROTOCOL_VERSION update/insert as before, and ensure
the setState call uses that merged params array when updating device; look for
symbols pollLocal, params, response.device/updatedDevice/latestDevice, newParams
and the final setState that writes device.params to implement this merge.

---

129-160: ⚠️ Potential issue | 🟡 Minor

Unknown-spec detection may over-report unsupported features.

This logic relies on exact code matching; Tuya payloads often expose equivalent data using different code names across sections, which can trigger false "partial support" and unnecessary issue creation prompts.

Based on learnings: In the Tuya integration for the Gladys Assistant repository, test fixtures preserve real-world inconsistencies (e.g., codes differing across specifications/status/properties sections) to ensure robust coverage of varying API payloads.

,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx` around lines 129 -
160, getUnknownSpecificationCodes currently compares codes by exact lowercased
strings which over-reports differences when Tuya uses variant naming; in
getUnknownSpecificationCodes normalize both knownCodes and specCodes more
robustly by: when building knownCodes from features (and specCodes from
specifications) trim and toLowerCase, also add canonical variants (strip common
separators like '_' and '-', remove numeric instance suffixes like trailing
digits or "_1"/"-1", and include the base token before any namespace delimiter
':'), then compare these canonical forms against getIgnoredCloudCodes
canonicalized the same way; update references to knownCodes, specCodes and the
final filter to use the canonical form so logically equivalent codes (e.g.,
"switch_1" vs "switch") are treated as matches rather than unknowns.

front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx (1)

292-307: ⚠️ Potential issue | 🟡 Minor

tuyaConfigured derivation should mirror trimmed save inputs.

updateConfiguration marks config as complete with whitespace-only values, while save-time logic trims and treats them as missing. This creates inconsistent UI state where "Save" button may be enabled but save will treat fields as empty.

,

💡 Suggested fix

   updateConfiguration = e => {
     const { name, value } = e.target;
     this.setState(prevState => {
       const nextState = { ...prevState, [name]: value };
-      const configured = !!(
-        nextState.tuyaEndpoint &&
-        nextState.tuyaAccessKey &&
-        nextState.tuyaSecretKey &&
-        nextState.tuyaAppAccountId
-      );
+      const isFilled = key => String(nextState[key] || '').trim().length > 0;
+      const configured =
+        isFilled('tuyaEndpoint') &&
+        isFilled('tuyaAccessKey') &&
+        isFilled('tuyaSecretKey') &&
+        isFilled('tuyaAppAccountId');
       return {
         [name]: value,
         tuyaConfigured: configured
       };
     });
   };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx` around lines
292 - 307, The updateConfiguration handler computes tuyaConfigured from raw
values allowing whitespace-only strings to count as configured; change its
derivation to use trimmed values so it mirrors save-time logic: in
updateConfiguration (method name) build nextState as you do, but when computing
configured check (nextState.tuyaEndpoint || '').trim(), (nextState.tuyaAccessKey
|| '').trim(), (nextState.tuyaSecretKey || '').trim(), and
(nextState.tuyaAppAccountId || '').trim() instead of the raw properties; still
persist the original value into state under [name], but set tuyaConfigured based
on those trimmed checks so the Save button state matches save-time validation.

server/test/services/tuya/lib/utils/tuya.deviceParams.test.js (1)

91-98: Add params assertions in partial/apply-existing scenarios to fully enforce sync contract.

These cases currently assert top-level fields but not the mirrored device.params entries. Adding those checks would better protect against regressions where field updates and param updates diverge.

Suggested assertion additions

   it('should keep protocol and product key when local info is partial', () => {
     const device = { ip: 'old', protocol_version: '3.3', product_key: 'pkey', params: [] };
     const localInfo = { ip: '2.2.2.2' };
     const updated = updateDiscoveredDeviceWithLocalInfo(device, localInfo);
+    const ipParam = updated.params.find((param) => param.name === DEVICE_PARAM_NAME.IP_ADDRESS);
+    const protocolParam = updated.params.find((param) => param.name === DEVICE_PARAM_NAME.PROTOCOL_VERSION);
+    const productKeyParam = updated.params.find((param) => param.name === DEVICE_PARAM_NAME.PRODUCT_KEY);
     expect(updated.ip).to.equal('2.2.2.2');
     expect(updated.protocol_version).to.equal('3.3');
     expect(updated.product_key).to.equal('pkey');
+    expect(ipParam.value).to.equal('2.2.2.2');
+    expect(protocolParam.value).to.equal('3.3');
+    expect(productKeyParam.value).to.equal('pkey');
   });

   it('should apply existing local params', () => {
     const device = { ip: 'old', protocol_version: '3.1', local_override: false, params: [] };
@@
     const updated = applyExistingLocalParams(device, existingDevice);
+    const ipParam = updated.params.find((param) => param.name === DEVICE_PARAM_NAME.IP_ADDRESS);
+    const protocolParam = updated.params.find((param) => param.name === DEVICE_PARAM_NAME.PROTOCOL_VERSION);
+    const overrideParam = updated.params.find((param) => param.name === DEVICE_PARAM_NAME.LOCAL_OVERRIDE);
     expect(updated.ip).to.equal('2.2.2.2');
     expect(updated.protocol_version).to.equal('3.3');
     expect(updated.local_override).to.equal(true);
+    expect(ipParam.value).to.equal('2.2.2.2');
+    expect(protocolParam.value).to.equal('3.3');
+    expect(overrideParam.value).to.equal(true);
   });

Also applies to: 111-124

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/utils/tuya.deviceParams.test.js` around lines
91 - 98, The test is missing assertions for device.params when local info is
partial; update the two tests that check "partial/apply-existing" behavior (the
one using updateDiscoveredDeviceWithLocalInfo around the lines shown and the
similar case at lines ~111-124) to also assert that corresponding entries in
updated.params reflect the top-level fields (ip, protocol_version, product_key)
remain/are updated accordingly, by locating updateDiscoveredDeviceWithLocalInfo
in the test and adding assertions that verify updated.params contains entries
with the expected ip, protocol_version and product_key values.

front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx (2)

694-699: Popup manipulation may cause issues in some browsers.

Setting popup.opener = null and manipulating popup.document before navigation can fail silently if the popup is blocked or if cross-origin restrictions apply. The code handles the !popup case but doesn't handle partial failures.

💡 Consider wrapping popup manipulation in try-catch

     const popup = window.open('about:blank', '_blank');
     if (popup) {
-      popup.opener = null;
-      popup.document.title = 'GitHub';
-      popup.document.body.innerText = 'Searching for existing issues...';
+      try {
+        popup.opener = null;
+        popup.document.title = 'GitHub';
+        popup.document.body.innerText = 'Searching for existing issues...';
+      } catch (e) {
+        // Some browsers may restrict document access
+      }
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx` around lines 694 -
699, The popup creation and direct DOM access (window.open -> popup, then
popup.opener and popup.document.*) can fail silently due to blockers or
cross-origin limits; wrap the popup manipulation and document writes in a
try-catch around the block that sets popup.opener, popup.document.title, and
popup.document.body.innerText (the code that runs after const popup =
window.open(...)), check that popup still exists and is not closed before
touching it, and if an exception or partial failure occurs fall back to safe
behavior such as navigating the popup via popup.location.href to the target URL
or opening the target in the current window/tab; ensure you reference and
protect the popup variable and the section that assigns popup.opener and
manipulates popup.document.

---

287-315: Consider surfacing GitHub API failures to the user.

When the GitHub search fails (Line 309), the error is thrown but may not always surface clearly in the UI. Users may be confused if the issue check silently fails and the flow proceeds with shouldOpenIssue = true.

💡 Consider adding user feedback for search failures

     } catch (error) {
       shouldOpenIssue = true;
+      // Optional: surface this to user so they know the duplicate check couldn't complete
+      console.warn('GitHub issue search failed, proceeding with issue creation', error);
     } finally {
       this.setState({ githubIssueChecking: false });
     }

Alternatively, you could set a state flag like githubIssueCheckFailed and show a brief warning in the UI.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx` around lines 287 -
315, The GitHub search error is currently thrown inside checkGithubIssueExists
which can leave the UI unaware; modify checkGithubIssueExists to catch
fetch/response/json errors (around the fetch/response.ok/json sections), log the
error, set/update a component state flag (e.g. githubIssueCheckFailed via a
setter you add) and return a sentinel (null/undefined) instead of throwing; then
update the caller logic that computes shouldOpenIssue to treat a null result as
"unknown" (don’t auto-open an issue) and render a brief UI warning when
githubIssueCheckFailed is true so users see the failure; keep existing cache
behavior (githubIssueCache and setGithubIssueCache) for successful responses.

server/test/services/tuya/lib/tuya.localScan.test.js (1)

8-31: Consider extracting a shared socket stub factory to reduce duplication.

The dgram/socket scaffolding is repeated across tests with small variations. A helper would reduce maintenance cost and make new scenarios easier to add safely.

Also applies to: 82-99, 150-165, 199-228, 255-275

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.localScan.test.js` around lines 8 - 31,
Extract the repeated socket scaffolding into a shared factory (e.g.,
createSocketStub / makeDgramStub) and replace the inline dgramStub definitions
with a call to that factory; the factory should return an object with
createSocket that pushes sockets into a provided sockets array and returns a
socket exposing on, bind (supporting both cb and options-as-cb), close, and
handlers properties to match the current tests that reference
dgramStub.createSocket, sockets, and socket.handlers. Update all duplicated
occurrences (the blocks around lines 82-99, 150-165, 199-228, 255-275) to use
the new factory so tests reuse the same stub shape and behavior.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

server/test/services/tuya/lib/device/tuya.localMapping.fixtures.test.js (1)

20-23: ⚠️ Potential issue | 🟡 Minor

Enforce the expected external_id shape before extracting code.

A value like "foo" still passes; asserting presence of : will fail fast on malformed fixtures instead of silently accepting them.

✅ Suggested tightening

       const resolvedMapping = currentDevice.features.reduce((accumulator, feature) => {
         expect(feature.external_id).to.be.a('string');
-        const code = String(feature.external_id)
+        expect(feature.external_id).to.include(':');
+        const code = feature.external_id
           .split(':')
           .pop();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/device/tuya.localMapping.fixtures.test.js`
around lines 20 - 23, The test currently extracts `code` by splitting
`feature.external_id` without asserting its expected shape; add an assertion
that `feature.external_id` contains a colon (e.g.,
expect(feature.external_id).to.include(':') or match a /:/ regex) before
performing the String(...).split(':').pop() extraction so malformed values like
"foo" fail fast; update the assertion around `feature.external_id` in the
tuya.localMapping.fixtures.test to verify the presence of ':' then proceed to
extract `code`.

server/services/tuya/lib/mappings/index.js (1)

70-88: Consider the match order behavior.

The matchDeviceType function returns true early when category or productId matches, before checking codes and keywords. This is likely intentional for performance, but be aware that Object.values(DEVICE_TYPE_INDEX).find() at line 227-228 will return the first matching type in iteration order.

Since LIST_DEVICE_TYPES = [SMART_SOCKET, SMART_METER] defines the order, SMART_SOCKET will be checked first. If a device matches multiple type definitions, the first match wins. This should be fine for the current device types but is worth noting if more types are added later.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/mappings/index.js` around lines 70 - 88, The current
matchDeviceType function returns true immediately when category or productId
matches, which lets Object.values(DEVICE_TYPE_INDEX).find(...) and
LIST_DEVICE_TYPES ordering determine which type wins; to avoid accidental
misclassification make matching stricter: update matchDeviceType to not
short-circuit on category/productId alone but instead treat those as positive
signals combined with the REQUIRED_CODES and KEYWORDS checks (i.e., evaluate
requiredCodes/modelName/keywords regardless of category/productId and only
return true when both the category/productId signal and the code/keyword checks
pass), or alternatively add a clear comment near matchDeviceType and the
LIST_DEVICE_TYPES/DEVICE_TYPE_INDEX usage documenting that the first match wins
and that LIST_DEVICE_TYPES defines priority so future developers are aware;
reference matchDeviceType, DEVICE_TYPE_INDEX, and LIST_DEVICE_TYPES when making
the change.

server/services/tuya/lib/tuya.poll.js (1)

194-221: Clarify local-config semantics to avoid misleading diagnostics.

hasLocalConfig currently mixes “credentials exist” with “local mode enabled”. This makes has_local logs harder to reason about and couples two concerns in branch conditions. Split into hasLocalCredentials and shouldUseLocal.

♻️ Proposed refactor

-  const hasLocalConfig = Boolean(ipAddress && localKey && protocolVersion && localOverride === true);
-  const requestedMode = localOverride === true ? 'local' : 'cloud';
+  const hasLocalCredentials = Boolean(ipAddress && localKey && protocolVersion);
+  const shouldUseLocal = localOverride === true;
+  const requestedMode = shouldUseLocal ? 'local' : 'cloud';
   logger.debug(
     `[Tuya][poll] device=${topic} requested=${requestedMode} has_local=${Boolean(
-      hasLocalConfig,
+      hasLocalCredentials,
     )} protocol=${protocolVersion || 'none'} ip=${ipAddress || 'none'}`,
   );
@@
-  if (localOverride === true && !hasLocalConfig) {
+  if (shouldUseLocal && !hasLocalCredentials) {
@@
-  if (hasLocalConfig) {
+  if (shouldUseLocal && hasLocalCredentials) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/tuya.poll.js` around lines 194 - 221, hasLocalConfig
conflates "credentials exist" with "local mode requested", causing confusing
logs and branch logic; split it into hasLocalCredentials = Boolean(ipAddress &&
localKey && protocolVersion) and shouldUseLocal = localOverride === true, then
update all uses: replace hasLocalConfig checks with hasLocalCredentials where
you only care about config presence, and use shouldUseLocal where you care
whether local mode was requested (e.g., computing requestedMode, fallback when
shouldUseLocal && !hasLocalCredentials, and deciding to enter local handling
under hasLocalCredentials && shouldUseLocal if appropriate); also change the
logger.debug field from has_local to hasLocalCredentials (or both as needed) and
rename localHandled/localChanged usage to reflect these clearer semantics
(ensure fallbackReason logic uses shouldUseLocal and hasLocalCredentials).

front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx (2)

560-569: Consider also disabling disconnect when already manually disconnected.

The button could be clicked when tuyaManuallyDisconnected is already true, which would trigger a redundant API call.

💡 Suggested enhancement

                      disabled={!state.tuyaConfigured || state.tuyaDisconnecting || state.tuyaConnecting}
+                      // Also consider: || state.tuyaManuallyDisconnected

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx` around lines
560 - 569, The disconnect button can be clicked when tuyaManuallyDisconnected is
already true, causing a redundant API call; update the disabled condition on the
button to also check state.tuyaManuallyDisconnected and additionally add a guard
at the start of the disconnect handler (this.disconnectFromCloud) to return
early if this.state.tuyaManuallyDisconnected is true so the UI and handler both
prevent duplicate disconnect attempts.

---

155-181: Sequential HTTP calls can be parallelized for better performance.

These seven independent variable saves are executed sequentially. Parallelizing them with Promise.all would significantly reduce save latency.

♻️ Suggested refactor

-      await this.props.httpClient.post('/api/v1/service/tuya/variable/TUYA_ENDPOINT', {
-        value: tuyaEndpoint
-      });
-
-      await this.props.httpClient.post('/api/v1/service/tuya/variable/TUYA_ACCESS_KEY', {
-        value: tuyaAccessKey
-      });
-
-      await this.props.httpClient.post('/api/v1/service/tuya/variable/TUYA_SECRET_KEY', {
-        value: tuyaSecretKey
-      });
-
-      await this.props.httpClient.post('/api/v1/service/tuya/variable/TUYA_APP_ACCOUNT_UID', {
-        value: tuyaAppAccountId
-      });
-
-      await this.props.httpClient.post('/api/v1/service/tuya/variable/TUYA_APP_USERNAME', {
-        value: tuyaAppUsername
-      });
-
-      await this.props.httpClient.post('/api/v1/service/tuya/variable/TUYA_LAST_CONNECTED_CONFIG_HASH', {
-        value: ''
-      });
-
-      await this.props.httpClient.post('/api/v1/service/tuya/variable/TUYA_MANUAL_DISCONNECT', {
-        value: 'false'
-      });
+      await Promise.all([
+        this.props.httpClient.post('/api/v1/service/tuya/variable/TUYA_ENDPOINT', { value: tuyaEndpoint }),
+        this.props.httpClient.post('/api/v1/service/tuya/variable/TUYA_ACCESS_KEY', { value: tuyaAccessKey }),
+        this.props.httpClient.post('/api/v1/service/tuya/variable/TUYA_SECRET_KEY', { value: tuyaSecretKey }),
+        this.props.httpClient.post('/api/v1/service/tuya/variable/TUYA_APP_ACCOUNT_UID', { value: tuyaAppAccountId }),
+        this.props.httpClient.post('/api/v1/service/tuya/variable/TUYA_APP_USERNAME', { value: tuyaAppUsername }),
+        this.props.httpClient.post('/api/v1/service/tuya/variable/TUYA_LAST_CONNECTED_CONFIG_HASH', { value: '' }),
+        this.props.httpClient.post('/api/v1/service/tuya/variable/TUYA_MANUAL_DISCONNECT', { value: 'false' })
+      ]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx` around lines
155 - 181, The seven independent this.props.httpClient.post calls in
SetupTab.jsx are executed sequentially; replace them by creating an array of
post promises (each calling this.props.httpClient.post with the same
endpoints/values shown: TUYA_ENDPOINT, TUYA_ACCESS_KEY, TUYA_SECRET_KEY,
TUYA_APP_ACCOUNT_UID, TUYA_APP_USERNAME, TUYA_LAST_CONNECTED_CONFIG_HASH (''),
TUYA_MANUAL_DISCONNECT ('false')) and await them with Promise.all so the
requests run in parallel; ensure you still await the Promise.all result and
propagate or handle any errors the same way as before.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx (1)

814-842: ⚠️ Potential issue | 🟠 Major

openEmptyGithubIssue still opens popup after async work and will be blocked by browsers.

Line 840 calls window.open after await checkGithubIssueExists(...) on line 826. Browsers enforce transient user activation and lose the user gesture once an await is hit, causing popup blockers to treat this as non-user-initiated.

Note: handleCreateGithubIssue was correctly fixed (opens popup at line 718 before any await), but this method was not.

💡 Suggested fix: Open the window synchronously before the async work

   openEmptyGithubIssue = async e => {
     if (e && e.preventDefault) {
       e.preventDefault();
     }
     const { githubIssuePayloadUrl, githubIssueChecking, githubIssueExists, githubIssueOpened, device } = this.state;
     if (!githubIssuePayloadUrl || githubIssueChecking || githubIssueExists || githubIssueOpened || !device) {
       return;
     }
     const issueTitle = buildIssueTitle(device);
+    const popup = window.open('about:blank', '_blank');
+    if (popup) {
+      popup.opener = null;
+      popup.document.title = 'GitHub';
+      popup.document.body.innerText = 'Searching for existing issues...';
+    }
     this.setState({ githubIssueChecking: true });
     let shouldOpenIssue = true;
     try {
       const exists = await checkGithubIssueExists(issueTitle);
       if (exists) {
         shouldOpenIssue = false;
         this.setState({ githubIssueExists: true });
       }
     } catch (error) {
       shouldOpenIssue = true;
     } finally {
       this.setState({ githubIssueChecking: false });
     }
     if (!shouldOpenIssue) {
+      if (popup && !popup.closed) {
+        popup.close();
+      }
       return;
     }
     const title = encodeURIComponent(issueTitle);
-    window.open(`${GITHUB_BASE_URL}?title=${title}`, '_blank');
+    if (popup && !popup.closed) {
+      popup.location.href = `${GITHUB_BASE_URL}?title=${title}`;
+    } else {
+      window.open(`${GITHUB_BASE_URL}?title=${title}`, '_blank');
+    }
     this.setState({ githubIssueOpened: true });
   };

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro